Skip to content

Conversation

@DrDaveD
Copy link
Collaborator

@DrDaveD DrDaveD commented Feb 24, 2017

This removes anything ligo-specific, and supports automounting on Debian for all repos.

@DrDaveD DrDaveD requested a review from bbockelm February 24, 2017 19:32
[Service]
Type=simple
ExecStart=/usr/sbin/cvmfs-config-osgd -p /run/cvmfs-config-osgd.pid /cvmfs/config-osg.opensciencegrid.org
PIDFile=/run/cvmfs-config-osgd.pid
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I think this should have a Requires statement for autofs. After only designates only the sequencing, not the dependency.
  • Might also be possible to put a requirement on the /cvmfs mount itself.
  • Automatic restart?
  • Is the PID file actually necessary for a service of type simple?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make it so we can restart this service whenever autofs is restarted?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should have a Requires statement for autofs. After only designates only the sequencing, not the dependency.

Added. It does not replace the After functionality, so need both.

Might also be possible to put a requirement on the /cvmfs mount itself.

I don't see how.

Automatic restart?

I don't know what you mean by that.

Is the PID file actually necessary for a service of type simple?

Oh, you're right. I assumed it was needed so systemd would be able to kill it, but it must keep track of the PID separately.

Can we make it so we can restart this service whenever autofs is restarted?

Oh, maybe that's what you meant by "Automatic restart?" Yes that would be good. As a matter of fact, the "Requires" and "After" already make this service restart with "systemctl restart autofs". They do not make it start after "systemctl stop autofs" (that stops cvmfs-config-osg) followed by "systemctl start autofs" (that does not start cvmfs-config-osg). For that, I believe a "WantedBy=autofs.service" in the [Install] section would normally work, but it doesn't, I think because there is not actually an autofs.service file in the autofs package (it is just inferred by /etc/init.d/autofs). It does however work to do "ln -s ../cvmfs-config-osg.service /lib/systemd/system/autofs.service.wants/cvmfs-config-osg.service", I will add that to the package.


Copyright:

Copyright (c) 2017, OSG
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The OSG is not a legal entity. Original copyright should go to the original author of the files.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok I changed it to Fermi National Accelerator Laboratory

mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/cvmfs/$cvmfsdir
done
for key in opensciencegrid.org.pub; do
install -D -m 444 "${key}" $RPM_BUILD_ROOT%{_sysconfdir}/cvmfs/keys/opensciencegrid.org
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the install get wrapped up into a Makefile? I.e., have two targets - one RedHat, one Debian?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, I did that.

@DrDaveD
Copy link
Collaborator Author

DrDaveD commented Feb 28, 2017

@bbockelm Please re-review, I addressed all comments.

@bbockelm
Copy link
Contributor

bbockelm commented May 4, 2017

Hi @DrDaveD -

Sorry, I lost track of this. Still ready for me to re-review?

Brian

@DrDaveD
Copy link
Collaborator Author

DrDaveD commented May 4, 2017

Yes, although I don't know if anybody will use the Debian version since the LIGO folks decided to go a different direction.

@bbockelm
Copy link
Contributor

bbockelm commented May 4, 2017

Yeah - we can overhaul the Debian stuff later. I'm inclined to just merge for now because it is indeed a good improvement for how we package on RHEL.

@bbockelm bbockelm merged commit ad67ff8 into opensciencegrid:master May 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants